Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Windows: Add --name option to pslist plugin #1077

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

adiego8
Copy link

@adiego8 adiego8 commented Jan 8, 2024

Add the capability to filter by name the windows.pslist output, where users can define the name of the process they would like to filter out in the process list.

The behavior of the filter is described below:

  • Filter is case-sensitive, e.g. Input --name Foo will not return a process named foo.
  • Filters full match only, e.g Input --name foo: return all processes named foo, it would not return foobaz.
  • Input --name foo bar: return all processes named foo and bar.

Described below the semantics when --pid filter is defined with --name:

  • Input --name foo --pid 123: return all processes named foo and all processes with pid 123, if these intersect, meaning the process 123 is also named foo, it will return only that one process. If these cases dont exist return the empty list.
  • Input --name foo bar --pid 123 456: return all processes named foo and bar and all processes with pid 123 and 456. The semantics are the same as previous point but for multiple names and pids.

All cases mentioned above were tested manually.

Add the capability to filter by name the windows.pslist output, where
users can define the name of the process they would like to filter out
in the process list.

The behavior of the filter is described below:

- Filter is case-sensitive, e.g. Input `--name Foo` will not return a
  process named foo.
- Filters full match only, e.g Input `--name foo`: return all processes named foo, it would not
  return foobaz.
- Input `--name foo bar`: return all processes named foo and
  bar.

Described below the semantics when `--pid` filter is defined with `--name`:

- Input `--name foo --pid 123`: return all processes named foo
  and all processes with pid 123, if these intersect, meaning the
  process 123 is also named foo, it will return only that one process.
  If these cases dont exist return the empty list.
- Input `--name foo bar --pid 123 456`: return all processes
  named foo and bar and all processes with pid 123 and 456. The
  semantics are the same as previous point but for multiple names and pids.

All cases mentioned above were tested manually.
@ikelos
Copy link
Member

ikelos commented Jan 15, 2024

Without having done a review, I'm wondering why this can't be completed with a tool such as grep? There really doesn't seem to be a significant use case and there's been no justification for why it should be so restrictive (full match only, case-senstive). There have been a number of these filtration flag requests recently and instead of having them dotted across different plugins added one at a time, I'd prefer to a) get them in sync with each other and b) know why they're necessary in volatility rather than piping the volatility output through another tool that can do the filtration better (and wouldn't then require us to look after them)?

@ikelos
Copy link
Member

ikelos commented Feb 6, 2024

Sorry I haven't had time to get back to this, but I owe you a better explanation of the questions I put to you. My concern is that the basic task we're trying to accomplish is to filter the output data of the plugin, which actually doesn't need to be something the plugin knows or cares about at all.

This proposed solution does that by adding a flag to the plugin explicitly to filter a single column. What that could lead to is fifty more plugins all defining their own flags for filtering results, and some even potentially making a flag for each column to filter (--name <thing --ImageFileName <blah> --ppid <foo> --offset <wibble> --wow64 False...). That doesn't scale well, and it's missing the fact that the display of data is something the UI should be handling, and the actual data returned is what the plugin should handle. One quick rule of thumb is to figure out whether Volumetric (our example web GUI) would benefit from this flag, or if there's another way of doing it. In the Web UI, you'd likely have a text box near a column, type in a some letters and expect the results to be filtered to just those rows. The --name flag would strike you as a strange thing to have to fill in before running the plugin, if you can just filter the output once everything's been generated. That suggests it's not the right solution for the problem, because it won't translate to other UIs and it'll crowd the parameter-space of plugins.

So if it's something the plugin needs to know (because maybe it could speed up the scans) then it can be a plugin flag, but if it's something that applies to outputting the data, that lives with UI (so it can be applied to all plugins and different UIs can decide to do it differently and the number of parameters for each plugin stay managable). So I won't be taking this PR as is, but I'm happy to help work on a solution for the CLI to do filtering, something similar to #1085 perhaps? It'll need a bit of design and planning, but once it's done it should be a simple and effective way to filter results without requiring each plugin to write its own code to do essentially the same thing.

If there are specific cases where the plugin could significantly change its behaviour (reducing the time required to scan, or processing the data to carry out the filtering) then we could still add those as parameters as well as the generic UI filtering mechanism. We should also look at #1072, it might be that could be better achieved by a simple output filter, or whether it's quicker for the plugin to know which things it needs to dive into and which it doesn't.

You might also want to consider how the functionality could be added to Volumetric, it would be very cool to have live/editable filters of the ?

I hope that explains my earlier questions and gives you some insight into the thinking behind the call, sorry I was so abrupt first time around, there were a deluge of new pull requests that all needed my attention but that's not a good excuse for my response.

@adiego8
Copy link
Author

adiego8 commented Feb 8, 2024

No problem at all @ikelos and thank you for your clear explanation. I was probably missing the bigger picture since I am getting familiar with the project. I followed the pattern of --pid, for example, that is currently declared in few plugins as you mentioned. Your idea is clear and I think your point is valid, there are few things that would require me some time to digest, like Volumetric which I am not familiar with, along with other functionalities. As you mentioned your filtering layer idea would require some planning, design and broader knowledge of the project I would add, but I would be happy to restructure this code to that layer once in develop, since you would not be accepting it as is, which is completely fine.

@ikelos
Copy link
Member

ikelos commented Feb 18, 2024

This should now be superseded by #1098, please test that branch and see if it resolves the same problem this was trying to resolve, and if so this PR can be closed.

@adiego8
Copy link
Author

adiego8 commented Feb 19, 2024

Sounds good! Will test #1098 to check out the feature, once in develop, will close this one. Thanks.

@ikelos
Copy link
Member

ikelos commented Feb 23, 2024

No problem, #1098 has now landed in develop, please take a look to see how I imagined implementing it (and feel free to offer suggestions/improvements, there's a list of things that could do with improving attached to the PR). 5:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants